feat(codex): add account picker visibility setting - #1096

Walkthrough

The change adds optional codexAccountPickerEnabled support. Populated namespace maps remain enabled by default. Explicit false hides picker rows. Malformed persisted values fail closed. Account-qualified routing remains unchanged.

Changes

Account picker control

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Config
  participant codexAccountPickerEnabled
  participant AccountQualifiedCatalog
  participant ConfiguredCodexAccount
  Config->>codexAccountPickerEnabled: Provide picker flag and namespace map
  codexAccountPickerEnabled-->>AccountQualifiedCatalog: Return picker visibility
  AccountQualifiedCatalog->>ConfiguredCodexAccount: Route account-qualified native model
  ConfiguredCodexAccount-->>AccountQualifiedCatalog: Preserve account, namespace, and authentication

Possibly related PRs

• lidge-jun/opencodex#426: Extends the account-qualified Codex namespace feature with picker visibility configuration.
• lidge-jun/opencodex#671: Covers routing of account-qualified namespace models controlled by this flag.
• lidge-jun/opencodex#1019: Modifies the same picker flag across namespace logic, configuration, types, and tests.

Suggested reviewers: wibias, lidge-jun

• X
• Mastodon
• Reddit
• LinkedIn

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Semantic propagation

• Concepts audited: codexAccountPickerEnabled (new optional config boolean + visibility resolver)
• Authoritative sources: src/types.ts (OcxConfig.codexAccountPickerEnabled doc) + src/config.ts (configSchema entry) — the schema is the persistence contract; getDefaultConfig() intentionally omits the key
• Producers and consumers checked: producers = loadConfig / configDiagnosticsFromRaw (persisted) and validateConfigCandidate (live writes); consumers = none yet by design (dormant resolver, src/codex/account-namespaces.ts codexAccountPickerEnabled); repo-wide rg confirms no production consumer — consistent with the declared non-goal
• Public/derived representations checked: persisted config.json key; getDefaultConfig() absence; warning strings; no CLI/API/UI/dashboard surface in this slice
• Material variant partitions checked: omitted + empty map → false; omitted + non-empty map → true; explicit true + non-empty → true; explicit false → false; malformed persisted → degrade to false; malformed/inherited/accessor live values → rejected without invoking getters
• Positive and negative assertions checked: all quadrants covered in tests/codex-account-namespaces.test.ts; round-trip + fail-closed + warning in tests/config.test.ts; routing independence in tests/router.test.ts (incl. codexAccountMode: "pool" and provider.authMode: "forward")
• Unmapped surfaces: none — no other representation of the new key exists in the repo
• Unproven equivalence assumptions: none
• Representation mismatches: none
• Variant coverage gaps: none
• Axis verdict: pass

Linked: #1019 (split source), #425 (parent issue); none auto-closed by this PR

Usefulness

Real, well-scoped foundation slice: it establishes the setting contract and pure default resolver that the catalog-convergence and management/dashboard slices will build on, and it locks the compatibility behavior (non-empty hand-written selector map stays picker-enabled when the field is omitted). No user-visible change yet — explicitly declared and consistent with the maintainer-requested split on #1019.

Bugs / correctness

• Method: bug-review.md — Bugbot: n/a (Codex host, complementary in-session); static: typecheck green; complementary lenses done (silent_failures, resource_leaks, edge_cases, boundary_conditions, api_compatibility, error_propagation, state_consistency, network_cancellation, parsing_serialization)
• Findings: none blocking. Probe evidence verified via verify-probe-coverage.mjs (valid: true): input-shape-evidence-semantics clean, lock-error-propagation n/a (no lock surface), recursion-termination n/a (no resolver recursion), test-honesty clean (all new assertions non-vacuous)
• Local tip validation (worktree at head 435ade0): bun run typecheck green; focused suite 168 pass / 6 fail — the 6 failures are pre-existing config.ts symlink tests that fail identically on base dev@e9d957bf with Windows EPERM (Developer-Mode symlink permission), not introduced by this PR; bun test tests/repo-hygiene.test.ts 11/11; bun run privacy:scan passes
• Fixed this session: none (foreign PR — no edits)

Security

• Scope reviewed: config schema/validation boundary, warnings/logging, no credential or outbound surface. ai-agent-security skill loaded per scope; decision tree → n/a (no MCP/tool-invocation/retrieval surface in this diff). Coverage matrix complete: authn/authz/injection/SSRF/IaC/crypto n/a (untouched); secrets/config done (privacy:scan passes, scanner hits are pre-existing test fixtures only); logging/privacy done (new warnings log only the key name + expected type, no values)
• Findings: none confirmed
• Fixed this session: none

Spec / standards

• Spec source: PR body (first slice of feat(codex): add account picker lifecycle settings #1019 after requested split) + linked Expose Codex accounts as model namespaces in the picker #425; explicit non-goals (selector initialization, catalog convergence, management API/dashboard behavior, docs → later PRs)
• Gaps: none. Every declared requirement is implemented and tested: field optional without fresh defaults; non-empty hand-written map stays enabled when omitted; malformed persisted values degrade to false without discarding providers/accounts/selectors; malformed/inherited/accessor live values rejected without invoking getters; exact account-qualified routing independent of picker visibility
• Standards: follows the repo's established degrade-don't-destroy pattern (codexAccountPriorities, claudeSubagentEffort, native-subagent fields), JSDoc on new public declarations, tests next to subsystem. No code smells worth acting on — the new helpers mirror the existing warning/degrade convention intentionally

Reviews

• Owners/maintainers: @Ingwannu reviewed head 62f33d7e — no code-level blocker; waiting on the repository ci check for head 435ade0. Informational, no code change requested; carried as the CI owner action below
• Bots: CodeRabbit actionable finding (assert full routing contract in the disabled-picker router test) fixed on head and confirmed addressed by CodeRabbit; Codex P2s (flag not wired into catalog yet; docs not updated) answered by the author with scoped-slice rationale — durable declines consistent with the declared non-goals

Base / CI

• Behind/conflicts: clean — head 435ade0 is exactly 1 commit ahead of dev@e9d957bf (current base), mergeable
• Required checks: enforce-target, label, hygiene, CodeRabbit green on head. Cross-platform CI (ci) and React Doctor runs are queued awaiting fork-PR workflow approval (first-time contributor; repo approval policy) — no ci check result exists on this head. GitHub shows mergeStateStatus: CLEAN only because no branch-protection/ruleset marks ci as required; the repository convention and the owner's review treat it as required
• Local tip compile/tests: typecheck + focused suite green (168 pass; 6 pre-existing Windows symlink failures reproduced on base)

Simplification (for the PR owner)

Simplify was explicitly requested. Bounded candidate pass complete — no edits made (foreign PR). Findings:

• No meaningful behavior-preserving simplifications worth applying. The PR is already minimal (177 insertions across 6 files) and its structure intentionally mirrors the repo's established degrade-pattern convention for optional config fields (codexAccountPriorities, claudeSubagentEffort, native-subagent fields): schema .catch() + load-time warning + live-write rejection + pure resolver. Collapsing those layers would diverge from the repo convention, not simplify it.
• The one marginal candidate: in tests/codex-account-namespaces.test.ts, the malformed-value loop [null, "false", 0, {}, []] tests resolver inputs the typed schema can never produce. Trimming to the schema-reachable values would reduce noise slightly, but the assertions are harmless and defensive. Optional, not recommended.

Gate

none (not draft/WIP/do-not-merge). The blocker is required-CI incompleteness (queued approval), which on this foreign PR is an owner action.

Bottom line

This is a small, honest, well-tested contract slice with no code-level blockers on bug/security/spec/standards or review threads. It should land once the repository ci suite runs green on head 435ade0. The single open item is a maintainer approving the queued Cross-platform CI workflow run — no code change required.